Skip to content

fix(scheduler): fence reservation release at the write boundary - #1576

Open
Kuang-xianxin wants to merge 1 commit into
TokenRhythm:mainfrom
Kuang-xianxin:fix/scheduler-reservation-release-fencing
Open

fix(scheduler): fence reservation release at the write boundary#1576
Kuang-xianxin wants to merge 1 commit into
TokenRhythm:mainfrom
Kuang-xianxin:fix/scheduler-reservation-release-fencing

Conversation

@Kuang-xianxin

@Kuang-xianxin Kuang-xianxin commented Sep 7, 2026

Copy link
Copy Markdown

Scope

Cancellation cleanup can recreate a deleted cron job or overwrite a newer reservation, pause, or configuration edit. release_reservation() currently checks ownership with get(), then writes the entire stale snapshot through save()'s upsert. Another SQLite connection can commit between those operations.

Scope boundary: make reservation release a conditional SQL UPDATE that checks the token at the write boundary and clears only reservation fields. Only RUNNING changes to PENDING; other current lifecycle states and configuration stay intact. Return whether the update matched a row.

Non-goals: changing scheduler APIs, schema, result-finalization behavior, or execution retry policy.

Branch

Base branch: main

Target exception: N/A

Issue

Linked issue: None

If None, reason: independently found during scheduler persistence review. Searches for reservation release and related scheduler PRs did not identify a corresponding fix.

Release Note

Release note: Prevent cancelled cron-job cleanup from restoring deleted jobs or overwriting concurrent reservation and configuration changes.

Tests

The regression uses a real file-backed SQLite database and two JobStore connections. The competing writer holds BEGIN IMMEDIATE; cleanup is allowed to reach its write boundary before the writer deletes the row, replaces ownership, pauses the job, or edits its configuration. The execute observer only coordinates arrival; SQL, locking, commits, and reads are real.

All four cases fail on the original implementation: deletion is undone and the other cases restore stale configuration. After the fix, the same cases pass with both native aiosqlite and the SQLite3 compatibility backend.

Ruff: uv run --no-sync --python 3.13 ruff check src tests passed.

Type checking: uv run --no-sync --python 3.13 mypy src/opensquilla --show-error-codes passed for 1,552 source files.

Pytest: uv run --no-sync --python 3.13 pytest tests/test_scheduler -q --tb=short188 passed. With OPENSQUILLA_FORCE_SQLITE3_BACKEND=1, the four new race cases also passed.

Broader offline-suite attempt: 1,473 passed, 233 skipped, 3 failed, stopped by --maxfail=3. All three failures are in tests/test_migration/test_opensquilla_home_migration.py: test_code_task_runs_are_left_in_source_for_preview_and_apply, test_apply_imports_home_with_transforms, and test_profile_import_preserves_unmodified_toml_bytes_and_comments. They report missing temporary layout-receipt paths on Windows. All three were reproduced unchanged in a detached baseline worktree at bf7fa0e1e96d3bd13fa123feba42cb275a953c23, using the same interpreter and baseline imports. The complete repository suite is not claimed to pass.

Build: npm --prefix opensquilla-webui run build (including TypeScript/architecture checks and verified artifact staging) and uv build --wheel --python 3.13 passed. Node.js 24.19.0 was available first on PATH; a prior staging attempt with the host's Node.js 22 runtime crashed, so staging was also checked directly with Node.js 24 before the successful full build.

Regression tests: added.

Notes: Windows, CPython 3.13.14. The default test path remains offline, deterministic, credential-free, and safe for forks.

Maintainer Live Check

Maintainer live check: no

Surface: N/A

This persistence change was validated with local SQLite files; no real model, channel, or deployed gateway was used.

Safety

Fixtures use synthetic jobs and temporary databases. No credentials, private transcripts, or AI session artifacts are included.

Third-Party Origin

Third-party origin: none

Documentation Changes

  • References point to repository files.
  • Examples use synthetic data and contain no secrets.
  • No public API or configuration changes require documentation updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant